-
Notifications
You must be signed in to change notification settings - Fork 5
Package Version pages #1547
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 10-03-add_getpackageversiondependencies_to_dapper_and_dapperfake
Are you sure you want to change the base?
Package Version pages #1547
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces package version pages with community-scoped and non-community routes, including loaders, clientLoaders, and shouldRevalidate logic. Adds tabs for Readme, Required (placeholder), and Versions, using Suspense/Await and shared components. Extends routing, breadcrumbs, and link libraries to support new v/{version} paths. Implements semver-based table sorting and common action/link components. Expands DapperTs and Thunderstore API with getPackageVersionDetails endpoint, request/response schemas, and fake data support. Updates utility functions, a package listing revalidation check, and dev proxy for /p paths. Exposes additional schema (packageTeamSchema) and link interfaces. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
895c5a8
to
4582600
Compare
bd91070
to
e6fba84
Compare
4582600
to
b1e3d73
Compare
e6fba84
to
0ef1a10
Compare
b1e3d73
to
e6817ad
Compare
0ef1a10
to
c9789f0
Compare
e6817ad
to
e6956ff
Compare
e6956ff
to
73cc06b
Compare
73cc06b
to
7a9bbdb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address coderabbit/graphite comments (either take the suggestions into account or reply to the comment with a reason why not, or just close/delete the conversations which are pure hallucinaition).
301a157
to
d6fba25
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cyberstorm-remix/app/p/packageListing.tsx (1)
165-175
: Restore the namespace comparison inshouldRevalidate
.
oldPath[3]
evaluates to the literal"p"
for routes shaped like/c/<community>/p/<namespace>/<package>/…
, so this condition now always passes regardless of namespace. Navigating between packages that share the samepackageId
but belong to different namespaces will therefore skip revalidation and render stale data. Please compare the namespace segment again (the previousoldPath[4]
/newPath[4]
check) or derive the segment index programmatically.- oldPath[3] === newPath[3] && + oldPath[4] === newPath[4] &&
🧹 Nitpick comments (6)
apps/cyberstorm-remix/app/p/packageVersion.tsx (2)
75-84
: Fetch in parallel to reduce TTFB.These three awaits are independent; run them concurrently.
- return { - communityId: params.communityId, - community: await dapper.getCommunity(params.communityId), - version: await dapper.getPackageVersionDetails( - params.namespaceId, - params.packageId, - params.packageVersion - ), - team: await dapper.getTeamDetails(params.namespaceId), - }; + const [community, version, team] = await Promise.all([ + dapper.getCommunity(params.communityId), + dapper.getPackageVersionDetails( + params.namespaceId, + params.packageId, + params.packageVersion + ), + dapper.getTeamDetails(params.namespaceId), + ]); + return { communityId: params.communityId, community, version, team };
182-183
: Memo uses stale inputs.Include version and team in deps.
- const versionAndTeamPromise = useMemo(() => Promise.all([version, team]), []); + const versionAndTeamPromise = useMemo( + () => Promise.all([version, team]), + [version, team] + );packages/cyberstorm/src/components/Links/LinkingProvider.tsx (1)
66-66
: Typo in comment (“bew”).Minor doc fix.
-// STEP 3 of adding bew link definitions: +// STEP 3 of adding new link definitions:apps/cyberstorm-remix/app/p/packageVersionWithoutCommunity.tsx (3)
70-77
: Fetch in parallel to reduce TTFB.These two awaits are independent; run them concurrently.
- return { - version: await dapper.getPackageVersionDetails( - params.namespaceId, - params.packageId, - params.packageVersion - ), - team: await dapper.getTeamDetails(params.namespaceId), - }; + const [version, team] = await Promise.all([ + dapper.getPackageVersionDetails( + params.namespaceId, + params.packageId, + params.packageVersion + ), + dapper.getTeamDetails(params.namespaceId), + ]); + return { version, team };
162-163
: Memo uses stale inputs.Include version and team in deps.
- const versionAndTeamPromise = useMemo(() => Promise.all([version, team]), []); + const versionAndTeamPromise = useMemo( + () => Promise.all([version, team]), + [version, team] + );
227-233
: Link to team page for consistency.Use a Team link (as in the community-scoped page).
- <span> - <NewIcon csMode="inline" noWrapper> - <FontAwesomeIcon icon={faUsers} /> - </NewIcon> - {resolvedValue.namespace} - </span> + <NewLink + primitiveType="cyberstormLink" + linkId="Team" + team={resolvedValue.namespace} + csVariant="cyber" + rootClasses="page-header__meta-item" + > + <NewIcon csMode="inline" noWrapper> + <FontAwesomeIcon icon={faUsers} /> + </NewIcon> + {resolvedValue.namespace} + </NewLink>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
apps/cyberstorm-remix/app/p/packageListing.tsx
(1 hunks)apps/cyberstorm-remix/app/p/packageVersion.tsx
(1 hunks)apps/cyberstorm-remix/app/p/packageVersionWithoutCommunity.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionRequired.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionWithoutCommunityRequired.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionWithoutCommunityVersions.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx
(5 hunks)apps/cyberstorm-remix/app/p/tabs/Versions/common.tsx
(1 hunks)apps/cyberstorm-remix/app/root.tsx
(2 hunks)apps/cyberstorm-remix/app/routes.ts
(1 hunks)apps/cyberstorm-remix/cyberstorm/utils/LinkLibrary.tsx
(1 hunks)apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts
(1 hunks)apps/cyberstorm-storybook/LinkLibrary.tsx
(1 hunks)packages/cyberstorm/src/components/Links/LinkingProvider.tsx
(2 hunks)packages/cyberstorm/src/components/Links/Links.tsx
(1 hunks)packages/cyberstorm/src/utils/utils.ts
(1 hunks)packages/dapper-fake/src/fakers/package.ts
(1 hunks)packages/dapper-fake/src/index.ts
(2 hunks)packages/dapper-ts/src/index.ts
(3 hunks)packages/dapper-ts/src/methods/packageVersion.ts
(1 hunks)packages/thunderstore-api/src/get/packageVersionDetails.ts
(1 hunks)packages/thunderstore-api/src/index.ts
(1 hunks)packages/thunderstore-api/src/schemas/objectSchemas.ts
(1 hunks)packages/thunderstore-api/src/schemas/requestSchemas.ts
(1 hunks)packages/thunderstore-api/src/schemas/responseSchemas.ts
(2 hunks)tools/ts-dev-proxy/nginx.conf
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/thunderstore-api/src/index.ts
- apps/cyberstorm-remix/cyberstorm/utils/LinkLibrary.tsx
- packages/cyberstorm/src/components/Links/Links.tsx
- apps/cyberstorm-storybook/LinkLibrary.tsx
- apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionRequired.tsx
- packages/thunderstore-api/src/schemas/requestSchemas.ts
- packages/thunderstore-api/src/schemas/responseSchemas.ts
- tools/ts-dev-proxy/nginx.conf
🧰 Additional context used
🧬 Code graph analysis (15)
packages/dapper-fake/src/index.ts (1)
packages/dapper-fake/src/fakers/package.ts (1)
getFakePackageVersionDetails
(161-224)
packages/dapper-ts/src/index.ts (1)
packages/dapper-ts/src/methods/packageVersion.ts (1)
getPackageVersionDetails
(5-23)
apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts (1)
packages/cyberstorm/src/index.ts (1)
isRecord
(105-105)
packages/thunderstore-api/src/get/packageVersionDetails.ts (4)
packages/thunderstore-api/src/index.ts (1)
ApiEndpointProps
(9-15)packages/thunderstore-api/src/schemas/requestSchemas.ts (1)
PackageVersionDetailsRequestParams
(177-179)packages/thunderstore-api/src/schemas/responseSchemas.ts (2)
PackageVersionDetailsResponseData
(130-132)packageVersionDetailsResponseDataSchema
(113-128)packages/thunderstore-api/src/apiFetch.ts (1)
apiFetch
(59-114)
apps/cyberstorm-remix/app/p/packageVersionWithoutCommunity.tsx (13)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionWithoutCommunityVersions.tsx (2)
loader
(27-47)clientLoader
(49-69)apps/cyberstorm-remix/app/p/packageListing.tsx (3)
loader
(92-115)clientLoader
(132-161)shouldRevalidate
(165-177)apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx (2)
loader
(12-34)clientLoader
(36-58)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)apps/cyberstorm-remix/app/root.tsx (1)
OutletContextShape
(85-90)apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts (1)
isPromise
(7-9)packages/cyberstorm/src/components/RelativeTime/RelativeTime.tsx (1)
RelativeTime
(16-28)apps/cyberstorm-remix/app/commonComponents/PageHeader/PageHeader.tsx (1)
PageHeader
(17-81)packages/dapper-ts/src/methods/team.ts (1)
getTeamDetails
(10-24)packages/dapper-ts/src/methods/packageVersion.ts (1)
getPackageVersionDetails
(5-23)packages/cyberstorm/src/utils/utils.ts (2)
formatInteger
(6-11)formatFileSize
(17-31)apps/cyberstorm-remix/app/commonComponents/CopyButton/CopyButton.tsx (1)
CopyButton
(12-48)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx (4)
apps/cyberstorm-remix/app/p/packageVersion.tsx (2)
loader
(60-87)clientLoader
(89-116)apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx (2)
loader
(27-48)clientLoader
(50-71)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)
packages/dapper-ts/src/methods/packageVersion.ts (2)
packages/dapper-ts/src/index.ts (1)
DapperTsInterface
(31-34)packages/thunderstore-api/src/get/packageVersionDetails.ts (1)
fetchPackageVersionDetails
(9-24)
packages/dapper-fake/src/fakers/package.ts (2)
packages/dapper-fake/src/fakers/utils.ts (3)
setSeed
(74-77)getFakeImg
(3-4)range
(69-69)packages/dapper-fake/src/fakers/team.ts (1)
getFakeTeamMembers
(23-26)
apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (2)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx (2)
loader
(27-48)clientLoader
(50-71)apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionWithoutCommunityVersions.tsx (2)
loader
(27-47)clientLoader
(49-69)
packages/cyberstorm/src/utils/utils.ts (1)
packages/cyberstorm/src/index.ts (1)
formatToDisplayName
(106-106)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)
apps/cyberstorm-remix/app/p/packageVersion.tsx (12)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx (2)
loader
(27-48)clientLoader
(50-71)apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx (2)
loader
(12-34)clientLoader
(36-58)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)apps/cyberstorm-remix/app/root.tsx (1)
OutletContextShape
(85-90)apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts (1)
isPromise
(7-9)packages/cyberstorm/src/components/RelativeTime/RelativeTime.tsx (1)
RelativeTime
(16-28)apps/cyberstorm-remix/app/commonComponents/PageHeader/PageHeader.tsx (1)
PageHeader
(17-81)packages/dapper-ts/src/methods/team.ts (1)
getTeamDetails
(10-24)packages/dapper-ts/src/methods/packageVersion.ts (1)
getPackageVersionDetails
(5-23)packages/cyberstorm/src/utils/utils.ts (2)
formatInteger
(6-11)formatFileSize
(17-31)apps/cyberstorm-remix/app/commonComponents/CopyButton/CopyButton.tsx (1)
CopyButton
(12-48)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx (6)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionWithoutCommunityVersions.tsx (3)
loader
(27-47)clientLoader
(49-69)Versions
(87-154)apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (3)
loader
(28-49)clientLoader
(51-72)Versions
(90-157)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts (1)
isSemver
(13-15)apps/cyberstorm-remix/app/p/tabs/Versions/common.tsx (3)
ModManagerBanner
(7-18)DownloadLink
(20-37)InstallLink
(39-55)
apps/cyberstorm-remix/app/p/tabs/Versions/common.tsx (1)
packages/cyberstorm/src/svg/svg.tsx (1)
ThunderstoreLogo
(1-11)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionWithoutCommunityVersions.tsx (8)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx (3)
loader
(27-48)clientLoader
(50-71)Versions
(89-156)apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (3)
loader
(28-49)clientLoader
(51-72)Versions
(90-157)apps/cyberstorm-remix/app/p/packageVersionWithoutCommunity.tsx (2)
loader
(60-80)clientLoader
(82-102)apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx (2)
loader
(12-34)clientLoader
(36-58)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts (1)
isSemver
(13-15)apps/cyberstorm-remix/app/p/tabs/Versions/common.tsx (3)
ModManagerBanner
(7-18)DownloadLink
(20-37)InstallLink
(39-55)
🪛 ast-grep (0.39.5)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx
[warning] 75-75: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx
[warning] 75-75: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx
[error] 76-76: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx
[error] 76-76: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 ESLint
apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx
[error] 12-12: 'versionsSchema' is defined but never used.
(@typescript-eslint/no-unused-vars)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Generate visual diffs
- GitHub Check: Test
- GitHub Check: CodeQL
🔇 Additional comments (13)
packages/dapper-fake/src/fakers/package.ts (1)
175-223
: Align fake package version payload with API schemaThe fake payload still returns a
dependencies
array while omitting the requiredinstall_url
andversion_number
fields that the realPackageVersionDetailsResponseData
schema exposes. Consumers typed against the schema (and the new UI) will end up with missing CTA URLs / version labels, and the extra field can break strict typing. Please mirror the schema exactly.- // Fake dependencies (reuse logic but need extra flags) - const dependencyCount = faker.number.int({ min: 0, max: 6 }); - const dependencies = await Promise.all( - range(dependencyCount).map(async () => { - const depSeed = `${seed}-dep-${faker.string.alpha(8)}`; - setSeed(depSeed); - const isActive = faker.datatype.boolean(0.8); - const removed = faker.datatype.boolean(0.1); - const unavailable = !removed && faker.datatype.boolean(0.05); - return { - description: isActive - ? faker.company.buzzPhrase() - : "This package has been removed.", - icon_url: isActive - ? faker.helpers.maybe(() => getFakeImg(256, 256), { - probability: 0.85, - }) ?? null - : null, - is_active: isActive, - name: faker.word.words(3).split(" ").join("_"), - namespace: faker.word.sample(), - version_number: getVersionNumber(), - is_removed: removed, - is_unavailable: unavailable, - }; - }) - ); + const dependency_count = faker.number.int({ min: 0, max: 6 }); @@ - dependencies, - dependency_count: dependencies.length, + dependency_count, + install_url: `ror2mm://v1/install/thunderstore.io/${namespace}/${name}/${version}/`, @@ - }; + version_number: version, + };packages/dapper-fake/src/index.ts (1)
36-36
: Fake package version payload misses contract fieldsWiring
getPackageVersionDetails
to the fake source still returns a shape withoutversion_number
andinstall_url
, while the real API and consuming UI expect them. Local dev/testing will trip over the mismatch (the new pages read those keys), so please update the faker to keep parity.--- a/packages/dapper-fake/src/fakers/package.ts +++ b/packages/dapper-fake/src/fakers/package.ts @@ return { description: faker.company.buzzPhrase(), download_count: faker.number.int({ min: 0, max: 5_000_000 }), icon_url: iconUrl, name, namespace, + version_number: version, size: faker.number.int({ min: 20_000, max: 4_000_000_000 }), @@ download_url: `https://thunderstore.io/package/download/${namespace}/${name}/${version}/`, + install_url: + faker.helpers.maybe( + () => `https://thunderstore.io/package/install/${namespace}/${name}/${version}/`, + { probability: 0.9 } + ) ?? null, full_version_name: `${namespace}-${name}-${version}`,apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx (1)
1-58
: Reiterating blocker: sanitize README HTML before renderingThis surfaces raw README HTML from the API straight into the DOM. A malicious README (or a compromised upstream) can ship arbitrary
<script>
content, giving us an immediate XSS vector. Please sanitize the HTML in both loaders and at render time before callingdangerouslySetInnerHTML
.-import { Suspense } from "react"; +import { Suspense } from "react"; +import DOMPurify from "isomorphic-dompurify"; @@ - return { - readme: await dapper.getPackageReadme( - params.namespaceId, - params.packageId, - params.packageVersion - ), - }; + const readme = await dapper.getPackageReadme( + params.namespaceId, + params.packageId, + params.packageVersion + ); + return { + readme: { ...readme, html: DOMPurify.sanitize(readme.html) }, + }; @@ - readme: dapper.getPackageReadme( - params.namespaceId, - params.packageId, - params.packageVersion - ), + readme: dapper + .getPackageReadme( + params.namespaceId, + params.packageId, + params.packageVersion + ) + .then((value) => ({ + ...value, + html: DOMPurify.sanitize(value.html), + })), @@ - dangerouslySetInnerHTML={{ __html: resolvedValue.html }} + dangerouslySetInnerHTML={{ + __html: DOMPurify.sanitize(resolvedValue.html), + }}Also applies to: 74-77
apps/cyberstorm-remix/app/p/packageVersion.tsx (4)
153-172
: Effect misses deps and lacks error handling.Add version to deps and handle rejected promises.
- useEffect(() => { + useEffect(() => { if (!startsHydrated.current && isHydrated) return; if (isPromise(version)) { - version.then((versionData) => { + version + .then((versionData) => { setFirstUploaded( <RelativeTime time={versionData.datetime_created} suppressHydrationWarning /> ); - }); + }) + .catch((err) => { + console.error("Failed to load version data", err); + }); } else { setFirstUploaded( <RelativeTime time={version.datetime_created} suppressHydrationWarning /> ); } - }, []); + }, [version]);
177-181
: Memo uses stale inputs.Include version and community in deps.
- const versionAndCommunityPromise = useMemo( - () => Promise.all([version, community]), - [] - ); + const versionAndCommunityPromise = useMemo( + () => Promise.all([version, community]), + [version, community] + );
198-203
: Guard missing base URL in og:url.Add a fallback to avoid “undefined/pathname” when env is absent.
- <meta - property="og:url" - content={`${ - getPublicEnvVariables(["VITE_BETA_SITE_URL"]) - .VITE_BETA_SITE_URL - }${location.pathname}`} - /> + <meta + property="og:url" + content={`${getPublicEnvVariables(["VITE_BETA_SITE_URL"]).VITE_BETA_SITE_URL ?? ""}${location.pathname}`} + />
<title> sets the document title; is ignored.
190-194
: Use <title>, not meta title.- <meta - title={`${formatToDisplayName( - resolvedValue[0].full_version_name - )} | Thunderstore - The ${resolvedValue[1].name} Mod Database`} - /> + <title>{`${formatToDisplayName( + resolvedValue[0].full_version_name + )} | Thunderstore - The ${resolvedValue[1].name} Mod Database`}</title>packages/cyberstorm/src/components/Links/LinkingProvider.tsx (1)
112-128
: New link types wiring looks correct.Interface and noop defaults added consistently.
Also applies to: 182-187
apps/cyberstorm-remix/app/p/packageVersionWithoutCommunity.tsx (5)
104-116
: Revalidation path segments look correct.Namespace (2), package (3), version (5).
138-157
: Effect misses deps and lacks error handling.Add version to deps and handle rejected promises.
- useEffect(() => { + useEffect(() => { if (!startsHydrated.current && isHydrated) return; if (isPromise(version)) { - version.then((versionData) => { + version + .then((versionData) => { setFirstUploaded( <RelativeTime time={versionData.datetime_created} suppressHydrationWarning /> ); - }); + }) + .catch((err) => { + console.error("Failed to load version data", err); + }); } else { setFirstUploaded( <RelativeTime time={version.datetime_created} suppressHydrationWarning /> ); } - }, []); + }, [version]);
178-183
: Guard missing base URL in og:url.Add a fallback to avoid “undefined/pathname” when env is absent.
- <meta - property="og:url" - content={`${ - getPublicEnvVariables(["VITE_BETA_SITE_URL"]) - .VITE_BETA_SITE_URL - }${location.pathname}`} - /> + <meta + property="og:url" + content={`${getPublicEnvVariables(["VITE_BETA_SITE_URL"]).VITE_BETA_SITE_URL ?? ""}${location.pathname}`} + />
<title> sets the document title; is ignored.
170-174
: Use <title>, not meta title.- <meta - title={`${formatToDisplayName( - resolvedValue[0].full_version_name - )} | ${resolvedValue[1].name}`} - /> + <title>{`${formatToDisplayName( + resolvedValue[0].full_version_name + )} | ${resolvedValue[1].name}`}</title>
256-265
: Fix HTML Popover attributes (camelCase won’t work).Use lowercase popovertarget/popovertargetaction on native button.
- <button - popoverTarget="packageDetailDrawer" - popoverTargetAction="show" + <button + popovertarget="packageDetailDrawer" + popovertargetaction="show" className="button button--variant--secondary button--size--medium package-listing__drawer-button" >Likely an incorrect or invalid review comment.
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx
Outdated
Show resolved
Hide resolved
function rowSemverCompare( | ||
a: TableRow, | ||
b: TableRow, | ||
columnMeta: TableCompareColumnMeta | ||
) { | ||
if (isSemver(String(a[0].sortValue)) && isSemver(String(b[0].sortValue))) { | ||
if (semverLt(String(a[0].sortValue), String(b[0].sortValue))) { | ||
return columnMeta.direction; | ||
} | ||
if (semverGt(String(a[0].sortValue), String(b[0].sortValue))) { | ||
return -columnMeta.direction; | ||
} | ||
} | ||
return 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this work? or do we not have semverCompare?
Also I think this function should probably be a more generic helper the comparison itself isn't really PackageVersion specific.
function rowSemverCompare(
a: TableRow,
b: TableRow,
columnMeta: TableCompareColumnMeta
) {
const av = String(a[0].sortValue);
const bv = String(b[0].sortValue);
if (isSemver(av) && isSemver(bv)) {
return semverCompare(av, bv) * columnMeta.direction;
}
return 0;
}
Or even just call semverCompare(av, bv) * columnMeta.direction;
direclty inline without the wrapper function if that works?
}; | ||
} | ||
|
||
function rowSemverCompare( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the same function again, should be reusable as stated in the other comment.
); | ||
} | ||
|
||
const columns: NewTableLabels = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This object seems to be the same as in PackageVersionVersion.tsx
. Could it be declared once somewhere?
How much differences are there between these two components, should one just extend the other to reduce duplicate code?
</div> | ||
</div> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This component and thepackageVersionWithoutCommunity should also be checked for unnecessary duplicate code. If it can't reasonably be reduced, please explain why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no more duplicate code
d6fba25
to
2c332c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (22)
tools/ts-dev-proxy/nginx.conf (1)
68-71
: Fix the location matcher to avoid unintended /package routes capture.The prefix match
/p
will also match/package
and/package/create
(lines 83-91), potentially causing routing conflicts. Uselocation ^~ /p/
to anchor to the/p/
path and prevent the matcher from catching other prefixes.Apply this diff:
- location /p { - proxy_pass http://host.docker.internal:3000/p; + location ^~ /p/ { + proxy_pass http://host.docker.internal:3000; proxy_set_header Host new.thunderstore.temp; }Note: Removing the trailing
/p
fromproxy_pass
ensures nginx performs correct URI substitution with the^~
modifier.packages/dapper-fake/src/index.ts (1)
14-15
: Missing fields in getFakePackageVersionDetails implementation.The wiring here is correct, but the implementation in
packages/dapper-fake/src/fakers/package.ts
is missingversion_number
andinstall_url
fields expected by the API schema. This issue has already been flagged in previous review comments.Also applies to: 36-36
packages/dapper-fake/src/fakers/package.ts (1)
160-224
: Schema mismatch in getFakePackageVersionDetails.The implementation is missing
version_number
andinstall_url
fields required by the API schema, and includes adependencies
array that is not part of the schema (onlydependency_count
should be present). These issues have already been flagged in previous review comments.apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionWithoutCommunityVersions.tsx (4)
65-132
: Consider reducing code duplication (duplicate concern).This component is nearly identical to
PackageVersionVersions.tsx
. Previous reviews already suggested extracting common logic to reduce duplication.
21-41
: Inconsistent return types in loader (duplicate concern).The success path returns a Promise from
dapper.getPackageVersions()
, but the error path returns a plain array. This type inconsistency was already flagged in previous reviews.
52-57
: Remove undefined communityId from return (duplicate concern).The non-community route doesn't have a
communityId
parameter, so returning it here results in undefined. Already flagged in previous reviews.
58-62
: Inconsistent return type in clientLoader error path (duplicate concern).Same issue as the server loader—error path returns array while success path returns Promise.
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx (3)
67-134
: Consider reducing code duplication (duplicate concern).This component duplicates logic from
PackageVersionWithoutCommunityVersions.tsx
. Previous reviews already suggested extracting common components.
21-42
: Inconsistent return types in loader (duplicate concern).Success path returns Promise, error path returns array. Same issue flagged in previous reviews for both community and non-community variants.
44-65
: Inconsistent return type in clientLoader (duplicate concern).Same issue as loader—error path type doesn't match success path. Already flagged in previous reviews.
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx (3)
12-34
: Sanitize README HTML to prevent XSS (duplicate concern).The loader returns unsanitized HTML that will be rendered via dangerouslySetInnerHTML. This is a critical XSS vulnerability already flagged in previous reviews.
36-58
: Sanitize README HTML in client loader (duplicate concern).Same XSS risk as the server loader—unsanitized HTML will be injected into the DOM.
75-77
: XSS risk: unsanitized HTML injection (duplicate concern).This dangerouslySetInnerHTML usage injects untrusted README HTML without sanitization. Already flagged in previous reviews with recommended fix using isomorphic-dompurify.
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx (2)
12-58
: Sanitize README HTML in both loaders (duplicate concern).Both loader and clientLoader return unsanitized HTML that will be injected via dangerouslySetInnerHTML. This critical XSS vulnerability was already flagged with a detailed fix in previous reviews.
76-76
: XSS risk at injection point (duplicate concern).dangerouslySetInnerHTML injects untrusted README HTML without sanitization. Already flagged in previous reviews with complete DOMPurify-based fix.
apps/cyberstorm-remix/app/p/packageVersion.tsx (5)
153-172
: Add missing dependency and error handling (duplicate concern).useEffect missing
version
in dependency array and lacks error handling for Promise rejection. Both issues already flagged in previous reviews.
177-182
: Missing dependencies in useMemo (duplicate concern).Both
useMemo
calls have empty dependency arrays but useversion
,community
, andteam
. Already flagged in previous reviews.
190-194
: Invalid meta title attribute (duplicate concern).Using
title
attribute on<meta>
tag is invalid. Should use<title>
element instead. Already flagged in previous reviews.
198-201
: Fix og:url construction (duplicate concern).
getPublicEnvVariables()
returns an object, not a string. Need to access.VITE_BETA_SITE_URL
property. Already flagged in previous reviews.
306-315
: Fix HTML popover attribute casing (duplicate concern).React popover attributes should be lowercase (
popovertarget
,popovertargetaction
) to work as valid HTML attributes. Already flagged in previous reviews.apps/cyberstorm-remix/app/p/packageVersionWithoutCommunity.tsx (2)
170-174
: Use<title>
instead of<meta title=...>
.
<meta title=...>
is not valid HTML. Use a proper<title>
element.Apply this diff:
- <meta - title={`${formatToDisplayName( - resolvedValue[0].full_version_name - )} | ${resolvedValue[1].name}`} - /> + <title>{`${formatToDisplayName( + resolvedValue[0].full_version_name + )} | ${resolvedValue[1].name}`}</title>
177-183
: Fix og:url construction (currently renders "[object Object]").
getPublicEnvVariables
returns an object, not a string. Extract theVITE_BETA_SITE_URL
property before interpolating.Apply this diff:
<meta property="og:url" - content={`${ - getPublicEnvVariables(["VITE_BETA_SITE_URL"]) - .VITE_BETA_SITE_URL - }${location.pathname}`} + content={`${getPublicEnvVariables(["VITE_BETA_SITE_URL"]).VITE_BETA_SITE_URL ?? ""}${location.pathname}`} />
🧹 Nitpick comments (1)
apps/cyberstorm-remix/app/root.tsx (1)
467-515
: LGTM with an optional refactor suggestion.The breadcrumb blocks are correct. However, the
packageVersionWithoutCommunityPage
block (lines 488-515) uses nested spans for each segment, which is verbose. You could simplify this by reducing the nesting, but it's not a functional issue.Optional refactor to reduce nesting:
- {packageVersionWithoutCommunityPage ? ( - <> - <span> - <span> - { - packageVersionWithoutCommunityPage.params - .namespaceId - } - </span> - </span> - <span> - <span> - { - packageVersionWithoutCommunityPage.params - .packageId - } - </span> - </span> - <span> - <span> - { - packageVersionWithoutCommunityPage.params - .packageVersion - } - </span> - </span> - </> - ) : null} + {packageVersionWithoutCommunityPage ? ( + <> + <span><span>{packageVersionWithoutCommunityPage.params.namespaceId}</span></span> + <span><span>{packageVersionWithoutCommunityPage.params.packageId}</span></span> + <span><span>{packageVersionWithoutCommunityPage.params.packageVersion}</span></span> + </> + ) : null}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
apps/cyberstorm-remix/app/p/packageListing.tsx
(1 hunks)apps/cyberstorm-remix/app/p/packageVersion.tsx
(1 hunks)apps/cyberstorm-remix/app/p/packageVersionWithoutCommunity.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionRequired.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionWithoutCommunityRequired.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionWithoutCommunityVersions.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx
(5 hunks)apps/cyberstorm-remix/app/p/tabs/Versions/common.tsx
(1 hunks)apps/cyberstorm-remix/app/root.tsx
(2 hunks)apps/cyberstorm-remix/app/routes.ts
(1 hunks)apps/cyberstorm-remix/cyberstorm/utils/LinkLibrary.tsx
(1 hunks)apps/cyberstorm-remix/cyberstorm/utils/semverCompare.ts
(1 hunks)apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts
(1 hunks)apps/storybook/LinkLibrary.tsx
(1 hunks)packages/cyberstorm/src/components/Links/LinkingProvider.tsx
(2 hunks)packages/cyberstorm/src/components/Links/Links.tsx
(1 hunks)packages/cyberstorm/src/utils/utils.ts
(1 hunks)packages/dapper-fake/src/fakers/package.ts
(1 hunks)packages/dapper-fake/src/index.ts
(2 hunks)packages/dapper-ts/src/index.ts
(3 hunks)packages/dapper-ts/src/methods/packageVersion.ts
(1 hunks)packages/thunderstore-api/src/get/packageVersionDetails.ts
(1 hunks)packages/thunderstore-api/src/index.ts
(1 hunks)packages/thunderstore-api/src/schemas/objectSchemas.ts
(1 hunks)packages/thunderstore-api/src/schemas/requestSchemas.ts
(1 hunks)packages/thunderstore-api/src/schemas/responseSchemas.ts
(2 hunks)tools/ts-dev-proxy/nginx.conf
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionWithoutCommunityRequired.tsx
- apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts
- apps/cyberstorm-remix/app/routes.ts
- packages/thunderstore-api/src/schemas/responseSchemas.ts
- packages/cyberstorm/src/utils/utils.ts
- packages/thunderstore-api/src/schemas/objectSchemas.ts
- apps/cyberstorm-remix/app/p/packageListing.tsx
- packages/dapper-ts/src/index.ts
- apps/cyberstorm-remix/app/p/tabs/Versions/common.tsx
🧰 Additional context used
🧬 Code graph analysis (15)
apps/storybook/LinkLibrary.tsx (1)
apps/cyberstorm-remix/cyberstorm/utils/LinkLibrary.tsx (1)
Link
(18-58)
packages/dapper-fake/src/index.ts (1)
packages/dapper-fake/src/fakers/package.ts (1)
getFakePackageVersionDetails
(161-224)
packages/dapper-ts/src/methods/packageVersion.ts (2)
packages/dapper-ts/src/index.ts (1)
DapperTsInterface
(31-34)packages/thunderstore-api/src/get/packageVersionDetails.ts (1)
fetchPackageVersionDetails
(9-24)
packages/dapper-fake/src/fakers/package.ts (2)
packages/dapper-fake/src/fakers/utils.ts (3)
setSeed
(74-77)getFakeImg
(3-4)range
(69-69)packages/dapper-fake/src/fakers/team.ts (1)
getFakeTeamMembers
(23-26)
apps/cyberstorm-remix/app/p/packageVersion.tsx (12)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx (2)
loader
(21-42)clientLoader
(44-65)apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx (2)
loader
(12-34)clientLoader
(36-58)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)apps/cyberstorm-remix/app/root.tsx (1)
OutletContextShape
(85-90)apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts (1)
isPromise
(7-9)packages/cyberstorm/src/components/RelativeTime/RelativeTime.tsx (1)
RelativeTime
(16-28)apps/cyberstorm-remix/app/commonComponents/PageHeader/PageHeader.tsx (1)
PageHeader
(17-81)packages/dapper-ts/src/methods/team.ts (1)
getTeamDetails
(10-24)packages/dapper-ts/src/methods/packageVersion.ts (1)
getPackageVersionDetails
(5-23)packages/cyberstorm/src/utils/utils.ts (2)
formatInteger
(6-11)formatFileSize
(17-31)apps/cyberstorm-remix/app/commonComponents/CopyButton/CopyButton.tsx (1)
CopyButton
(12-48)
apps/cyberstorm-remix/cyberstorm/utils/semverCompare.ts (1)
apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts (1)
isSemver
(13-15)
packages/cyberstorm/src/components/Links/LinkingProvider.tsx (1)
.yarn/releases/yarn-1.19.0.cjs (1)
noop
(2200-2200)
apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (2)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx (3)
loader
(21-42)clientLoader
(44-65)Versions
(67-134)apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionWithoutCommunityVersions.tsx (3)
loader
(21-41)clientLoader
(43-63)Versions
(65-132)
apps/cyberstorm-remix/cyberstorm/utils/LinkLibrary.tsx (2)
apps/storybook/LinkLibrary.tsx (1)
Link
(28-59)packages/cyberstorm/src/index.ts (1)
Link
(49-49)
packages/thunderstore-api/src/get/packageVersionDetails.ts (4)
packages/thunderstore-api/src/index.ts (1)
ApiEndpointProps
(9-15)packages/thunderstore-api/src/schemas/requestSchemas.ts (1)
PackageVersionDetailsRequestParams
(177-179)packages/thunderstore-api/src/schemas/responseSchemas.ts (2)
PackageVersionDetailsResponseData
(130-132)packageVersionDetailsResponseDataSchema
(113-128)packages/thunderstore-api/src/apiFetch.ts (1)
apiFetch
(59-114)
apps/cyberstorm-remix/app/p/packageVersionWithoutCommunity.tsx (11)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionWithoutCommunityVersions.tsx (2)
loader
(21-41)clientLoader
(43-63)apps/cyberstorm-remix/app/p/packageListing.tsx (2)
loader
(92-115)clientLoader
(132-161)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts (1)
isPromise
(7-9)packages/cyberstorm/src/components/RelativeTime/RelativeTime.tsx (1)
RelativeTime
(16-28)apps/cyberstorm-remix/app/commonComponents/PageHeader/PageHeader.tsx (1)
PageHeader
(17-81)packages/dapper-ts/src/methods/team.ts (1)
getTeamDetails
(10-24)packages/dapper-ts/src/methods/packageVersion.ts (1)
getPackageVersionDetails
(5-23)packages/cyberstorm/src/utils/utils.ts (2)
formatInteger
(6-11)formatFileSize
(17-31)apps/cyberstorm-remix/app/commonComponents/CopyButton/CopyButton.tsx (1)
CopyButton
(12-48)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionWithoutCommunityVersions.tsx (6)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx (3)
loader
(21-42)clientLoader
(44-65)Versions
(67-134)apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (4)
loader
(21-42)clientLoader
(44-65)Versions
(67-134)columns
(136-153)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)apps/cyberstorm-remix/app/p/tabs/Versions/common.tsx (3)
ModManagerBanner
(7-18)DownloadLink
(20-37)InstallLink
(39-55)apps/cyberstorm-remix/cyberstorm/utils/semverCompare.ts (1)
rowSemverCompare
(6-19)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx (5)
apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (4)
loader
(21-42)clientLoader
(44-65)Versions
(67-134)columns
(136-153)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)apps/cyberstorm-remix/app/p/tabs/Versions/common.tsx (3)
ModManagerBanner
(7-18)DownloadLink
(20-37)InstallLink
(39-55)apps/cyberstorm-remix/cyberstorm/utils/semverCompare.ts (1)
rowSemverCompare
(6-19)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx (4)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionVersions.tsx (2)
loader
(21-42)clientLoader
(44-65)apps/cyberstorm-remix/app/p/packageVersion.tsx (2)
loader
(60-87)clientLoader
(89-116)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx (4)
apps/cyberstorm-remix/app/p/tabs/Versions/PackageVersionWithoutCommunityVersions.tsx (2)
loader
(21-41)clientLoader
(43-63)apps/cyberstorm-remix/app/p/packageVersionWithoutCommunity.tsx (2)
loader
(60-80)clientLoader
(82-102)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
getPublicEnvVariables
(19-48)getSessionTools
(50-67)packages/dapper-ts/src/index.ts (1)
DapperTs
(36-89)
🪛 ast-grep (0.39.5)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx
[warning] 75-75: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx
[warning] 75-75: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionReadme.tsx
[error] 76-76: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
apps/cyberstorm-remix/app/p/tabs/Readme/PackageVersionWithoutCommunityReadme.tsx
[error] 76-76: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Generate visual diffs
- GitHub Check: Test
🔇 Additional comments (26)
apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionRequired.tsx (1)
69-74
: Active implementation is clean.The placeholder component is straightforward and clearly communicates the blocked status. No issues with the active code.
packages/thunderstore-api/src/index.ts (1)
37-37
: LGTM!The export follows the established pattern and is correctly positioned alphabetically.
packages/cyberstorm/src/components/Links/Links.tsx (1)
43-47
: LGTM!The new link identifiers follow the established naming conventions and cleanly extend the API surface.
apps/cyberstorm-remix/cyberstorm/utils/semverCompare.ts (1)
6-19
: LGTM with a note on edge case behavior.The logic correctly compares valid semver strings and respects the sort direction. Returning
0
for invalid semver values means those rows maintain their original order (stable sort). This is acceptable if all version columns are expected to contain valid semver strings.packages/thunderstore-api/src/schemas/requestSchemas.ts (1)
170-179
: LGTM!The schema definition is well-formed and follows the established pattern for request parameter schemas.
apps/cyberstorm-remix/app/root.tsx (2)
274-277
: LGTM!The new page matches follow the established pattern.
450-459
: LGTM!The community breadcrumb logic correctly includes the new
packageVersionPage
in the conditions.apps/storybook/LinkLibrary.tsx (1)
167-201
: LGTM!The new link mappings follow the established pattern and correctly implement the URL patterns for community-scoped and non-community package version routes.
packages/dapper-ts/src/methods/packageVersion.ts (1)
5-23
: LGTM!Clean delegation to the API layer with proper parameter mapping.
packages/thunderstore-api/src/get/packageVersionDetails.ts (1)
9-24
: LGTM!Proper API endpoint implementation with schema validation.
apps/cyberstorm-remix/cyberstorm/utils/LinkLibrary.tsx (1)
162-196
: LGTM!New link mappings follow the established pattern and correctly construct version-specific routes.
apps/cyberstorm-remix/app/p/packageVersion.tsx (2)
60-116
: LGTM!Loaders correctly fetch version, community, and team data with proper 404 handling.
118-131
: LGTM!shouldRevalidate logic correctly prevents unnecessary revalidation when staying on the same package version.
apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (5)
8-8
: LGTM!Import additions for NewLink, shared components from common module, and rowSemverCompare are correct and align with the updated UI requirements.
Also applies to: 18-19
21-42
: LGTM!Loader correctly requires communityId and returns it alongside namespaceId/packageId for downstream NewLink usage.
44-65
: LGTM!ClientLoader mirrors the server-side loader structure and returns consistent data shape.
67-134
: LGTM!Component correctly destructures communityId and uses it in NewLink components to generate package version routes. Semver-aware sorting via rowSemverCompare on column 0 is appropriate for the Version column.
136-153
: LGTM!Exporting columns enables reuse across related components.
packages/cyberstorm/src/components/Links/LinkingProvider.tsx (2)
113-128
: LGTM!New link methods follow existing patterns and correctly define community and non-community variants with appropriate prop types.
182-186
: LGTM!Noop implementations for the new link methods are consistent with the default library pattern.
apps/cyberstorm-remix/app/p/packageVersionWithoutCommunity.tsx (6)
60-80
: LGTM!Loader correctly fetches version and team details and throws 404 on missing params.
82-102
: LGTM!ClientLoader mirrors the server-side loader and returns consistent promise-based data.
104-116
: LGTM!shouldRevalidate correctly compares namespace (index 2), package (index 3), and version (index 5) segments. Past review comment is outdated—the code already uses the correct indices.
205-417
: LGTM!Component body correctly implements responsive layout with drawer, tabs navigation, and sidebar. NewLink components use appropriate linkIds for non-community routes.
419-452
: LGTM!Actions component is correctly memoized and conditionally renders donation button based on team data.
454-492
: LGTM!packageMeta function correctly renders metadata with appropriate formatting and includes copy-to-clipboard for dependency string.
apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionRequired.tsx
Outdated
Show resolved
Hide resolved
Add function for using the endpoint and related schemas
Add packageVersion method for Dapper and DapperFake
This enables componens like NewLink to auto resolve links and it's used props, with the linkId prop. (the method name in LinkLibrary interface)
These pages are more or less the same as PackageListing pages/tabs. The biggest difference being the ommited Team and Package Listing related information as it's not relevant when viewing a package. The "Required" is mostly commented out, because the endpoint isn't yet available. (even for local development testing)
These pages are and should be exactly the same, not including community related features and functionalities. The reason these have to be their own files is that, react-router doesn't support using the same files for two routes at the same time. The "Required" is mostly commented out, because the endpoint isn't yet available. (even for local development testing)
Note that the ones that do not require a community to fetch the package version data, are not under the /c/ path
So that no-community-needing package pages are accessible through the proxy
…Version related pages
2c332c4
to
31096e0
Compare
Look at the commits for more information